-
Notifications
You must be signed in to change notification settings - Fork 614
Use Puppet-Datatype Sensitive for Passwords #1279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Puppet-Datatype Sensitive for Passwords #1279
Conversation
cocker-cc
commented
Jun 12, 2021
- add Parameter "sensitive" to Function postgresql_password to decide if its Returnvalue should be of Datatype Sensitive
- let defined Type postgresql::server::role accept Datatype Sensitive for $password_hash
- let defined Type postgresql::server::db accept Datatype Sensitive for $password
- let Class postgresql::server accept Datatype Sensitive for $postgres_password
- let defined Type postgresql::validate_db_connection accept Sensitive for $database_password
manifests/server.pp
Outdated
@@ -83,7 +83,7 @@ | |||
# @param extra_systemd_config Adds extra config to systemd config file, can for instance be used to add extra openfiles. This can be a multi line string | |||
# | |||
class postgresql::server ( | |||
$postgres_password = undef, | |||
Optional[Variant[String, Sensitive[String]]] $postgres_password = undef, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking about compatibility here: could a user specify an integer password?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean Compatibility to postgresql::postgresql_password
?
required_param 'Variant[String[1], Sensitive[String[1]], Integer]', :password
I don't see Integer
as valid there neither.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to other work I didn't have time to look into this for a while. How is it not valid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my Understanding a Password is a String, not an Integer. But anyway, I adapted your Suggestion:
Optional[Variant[String[1], Sensitive[String[1]], Integer]] $postgres_password = undef,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for your understanding, you can have a numeric password and if you don't quote it in YAML (with Hiera for example) it becomes Integer
. I agree it's not that likely, but it can happen and we should be consistent.
a3f2b12
to
ff0b4ae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given what we've seen in theforeman/foreman, I think a function stdlib::unwrap_sensitive
may actually make sense, given how common it is that you need to unwrap something. What do you think?
I have this PR pending. If and when this is accepted, your suggested Function is not needed anymore. Perhaps You could vote for this PR. |
ff0b4ae
to
4ec009d
Compare
@ekohl Now that the associated pr has been merged what are your thoughts on this? |
- add Parameter "sensitive" to Function postgresql_password to decide if its Returnvalue should be of Datatype Sensitive - let defined Type postgresql::server::role accept Datatype Sensitive for $password_hash - let defined Type postgresql::server::db accept Datatype Sensitive for $password - let Class postgresql::server accept Datatype Sensitive for $postgres_password - let defined Type postgresql::validate_db_connection accept Sensitive for $database_password
4ec009d
to
d878d13
Compare
@cocker-cc Everything LGTM so gonna go ahead and merge once the test's go green. |
postgresql::postgresql_password is a functionthat may have no external impact to Forge modules. postgresql::server is a classthat may have no external impact to Forge modules. postgresql::server::db is a typethat may have no external impact to Forge modules. postgresql::server::default_privileges is a typethat may have no external impact to Forge modules. postgresql::server::passwd is a classthat may have no external impact to Forge modules. postgresql::server::role is a typethat may have no external impact to Forge modules. postgresql::validate_db_connection is a typethat may have no external impact to Forge modules. This module is declared in 70 of 578 indexed public
|
Just quickly closed and reopened to rekick the test's. No need to worry |
…_Sensitive Use Puppet-Datatype Sensitive for Passwords